Skip to content

fix(e2e+ui): clear the run-4012 matrix remainder — SW navigation hijack, WebKit quirks, Firefox tab order, CIWA alias#996

Merged
BigSimmo merged 2 commits into
mainfrom
claude/clinical-kb-pwa-review-asi3wb
Jul 20, 2026
Merged

fix(e2e+ui): clear the run-4012 matrix remainder — SW navigation hijack, WebKit quirks, Firefox tab order, CIWA alias#996
BigSimmo merged 2 commits into
mainfrom
claude/clinical-kb-pwa-review-asi3wb

Conversation

@BigSimmo

@BigSimmo BigSimmo commented Jul 20, 2026

Copy link
Copy Markdown
Owner

Summary

Seven root-caused fixes for the browser-matrix remainder (run 4012 baseline: 28 failures) plus the canary CIWA gap — every finding agent-diagnosed with probe or arithmetic evidence before any code was written:

  • Service-worker navigation hijack (the big one — playwright.config.ts + tests/ui-pwa.spec.ts): in production builds public/sw.js registers in every matrix test (pwa-lifecycle.tsx registers whenever NODE_ENV === "production"), clients.claim()s the page seconds in, and serves every subsequent navigation. A Chromium probe proved SW-served reloads bypass page.route interception entirely, and Playwright-Firefox wedges on exactly this path — its only two page.reload() calls in the whole suite are the two 60-second ui-smoke hangs (2206 and the @critical 2932). Fix: serviceWorkers: "block" in the shared config; ui-pwa.spec.ts — the one spec whose subject is the worker — opts back in with test.use({ serviceWorkers: "allow" }), and its offline/CacheStorage privacy journey passes locally under the opt-in.
  • WebKit mode-menu dismissal (src/components/clinical-dashboard/master-search-header.tsx): dismissal relied solely on a wrapper focusout; WebKit's sequential focus navigation can move focus nowhere (links are excluded from its Tab order — the previous tabbable is the skip link) or wrap backward into the open menu, so no wrapper-escaping focusout ever fires. Fix: close on Tab in the trigger's keydown handler (APG menu-button pattern; arrow keys remain the entry). Agent-verified no existing test depends on the old forward-Tab behavior; pointer/Escape/blur paths unchanged.
  • WebKit forced-colors capability (tests/ui-accessibility.spec.ts): WebKit has never implemented forced-colors; Playwright plumbs the emulation flag but the @media (forced-colors: active) remap under test cannot engage. Fix: webkit test.skip in the repo's established capability-guard style. Chromium/Firefox keep full coverage; the sibling generic forced-colors usability test stays unguarded (it passed on WebKit).
  • WebKit phantom axe contrast (tests/ui-formulation.spec.ts): the reported #b5bbc2 at 1.93:1 is byte-exact the 0.4-opacity disabled composite of --text-muted on white — but the Previous button is provably enabled at scan time and axe skips truly disabled controls. Only consistent mechanism: WebKit serving a stale :disabled computed style after React re-enables the button. Fix: pin toBeEnabled() + toHaveCSS("opacity", "1") before the scan — heals the stale style or converts the failure into direct proof (fallback documented in-test). WCAG 1.4.3 exempts disabled controls, so no design change is warranted.
  • Firefox tab order (tests/ui-smoke.spec.ts): Firefox includes scrollable containers in the tab order; the sheet body (overflow-y-auto, measured genuinely overflowing at the test viewport) sits between Close and "New chat" in DOM order. Fix: conditional step-over in the test (an app-side tabIndex={-1} on the shared Sheet primitive would remove Firefox's keyboard-scroll affordance — a product decision deliberately not taken here).
  • Canary CIWA content gate (scripts/eval-retrieval.ts): canary runs Save Codex local changes #50 and Refactor monolithic front‑end and decouple domain logic #51 rank the CIWA-Ar dosing-table region in the top 5, yet the content gate reports "ciwa OR score OR threshold" missing — because textContainsClinicalTerm is whitespace-delimited and the scale's written name is the hyphenated token "CIWA-Ar", which the bare fixture term can never match. Fix: ciwa: ["ciwa", "ciwa-ar"] in clinicalContentAliases — the eval's own documented drift-absorption mechanism (plan-authorized fixture-alias route). The clinical substance of the expectation is unchanged.
  • Shared stub helper (tests/helpers/zero-touch.ts + six specs): extracts test(e2e): report real zero-touch capability to WebKit for command-surface specs #995's six inline maxTouchPoints stubs into one helper — the CodeRabbit review follow-up.

Verification

  • Targeted vitest (eval-retrieval, search-command-surface) — 25/25
  • npm run test — 3012 passed; sole failure is the long-baselined container-only pdf-extraction-budget artifact
  • npm run verify:cheap — all static checks, lint, typecheck green; same single artifact in the test step
  • npm run build + client-bundle secret scan — pass
  • Chromium ui-pwa under the new SW config — the cold-offline/CacheStorage privacy journey passes with the allow opt-in; the installability test shows only the known container-only in-incognito artifact (hosted-CI-green since feat(pwa): retirement kill-switch, offline-version binding guard, and dev teardown flag #826)

UI verification not run: the local Chromium UI suites carry documented container artifacts (16/20 ui-universal-search failures identical on unmodified main; Playwright browser-build mismatch after the dependabot bump) — hosted ui-critical/ui-advisory on this PR plus the post-merge matrix dispatch are the authoritative UI verdicts. npm run eval:retrieval:quality cannot run locally (live keys exist only in Actions); the alias change is validated by the next canary run — the failing case's evidence chain (#50/#51 logs) is recorded in the ledger row.

Risk and rollout

  • Risk: low-moderate, dominated by the config change. serviceWorkers: "block" returns every non-PWA test to the pre-July-17 (pre-sw.js) behavior they were written under; the worker's own coverage is preserved via the ui-pwa opt-in and the untouched Vitest SW policy suites. The mode-menu Tab-close is a one-branch keyboard refinement matching the APG pattern; remaining changes are test-side or the eval fixture alias.
  • Rollback: plain revert of the content commit; the ledger commit is documentation-only.
  • Provider or production effects: None from merging. The post-merge matrix dispatch is $0; the alias is exercised by the next scheduled canary.

Clinical Governance Preflight

  • Source-backed claims still require linked source verification before clinical use
  • No patient-identifiable document workflow was introduced or expanded without explicit governance approval
  • Supabase target remains Clinical KB Database (sjrfecxgysukkwxsowpy)
  • Service-role keys and private document access remain server-only
  • Demo/synthetic content remains clearly separated from real clinical sources
  • Source metadata, review status, and outdated/unknown-source behavior remain conservative (the eval alias recognizes the CIWA-Ar scale's written name; it does not relax any expectation, threshold, or ranking behavior)
  • Deployment classification/TGA SaMD impact was checked when clinical decision-support behavior changed (mode-menu keyboard dismissal is navigation chrome; no clinical output surface changed)

Notes

  • Expected next-matrix delta vs the 28-failure baseline: the two Firefox reload hangs, the Firefox tab-order case, the WebKit menu/forced-colors/formulation trio, plus whatever the test(e2e): report real zero-touch capability to WebKit for command-surface specs #995 stub already cleared. Explicitly not claimed: ui-stress:409, ui-tools:2087, and the WebKit answer-flow subset — the post-merge dispatch measures those; anything surviving gets its own diagnosis round.

🤖 Generated with Claude Code

https://claude.ai/code/session_01EXsJcLrbZUXwnBeG91cVo9


Generated by Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Improved desktop keyboard navigation so mode menus close correctly when tabbing away.
    • Improved focus handling in mobile search flows, including clipped layouts and scrollable content.
    • Corrected clinical content matching for CIWA-A terminology.
    • Improved validation of step-navigation controls and accessibility behavior across browsers.
  • Tests

    • Increased cross-browser reliability for PWA navigation, touch detection, forced-colors checks, and UI smoke tests.

claude added 2 commits July 20, 2026 15:19
…ck, WebKit focus/forced-colors/stale-style, Firefox tab order, CIWA term alias

Seven fixes from the run-4012 triage, each root-caused:

- playwright.config.ts: serviceWorkers 'block' suite-wide (+ ui-pwa.spec.ts
  'allow' opt-in). In production builds public/sw.js registers in every
  test, claims the page (clients.claim) and serves every navigation —
  probe-proven to bypass route interception for navigations on Chromium
  and to wedge Playwright-Firefox's reload path under an active route
  (both ui-smoke reload hangs). Only ui-pwa exercises the worker itself;
  its offline/CacheStorage journey passes under the opt-in.
- master-search-header.tsx: close the desktop mode menu on Tab from the
  trigger (APG menu-button pattern). WebKit's sequential focus navigation
  can fail to deliver a wrapper-escaping focusout (links excluded from
  its Tab order; backward wrap into the menu), leaving the menu open.
- ui-accessibility.spec.ts: webkit skip for the forced-colors token test —
  WebKit has no forced-colors implementation, so the media remap under
  test cannot engage there.
- ui-formulation.spec.ts: pin the step-nav Previous button enabled with
  opacity 1 before the axe scan — WebKit can serve a stale :disabled
  computed style whose 0.4 opacity axe folds into a phantom contrast
  violation for a WCAG-1.4.3-exempt state.
- ui-smoke.spec.ts: step over Firefox's scrollable-container tab stop
  (the sheet body sits between Close and 'New chat' in DOM order).
- scripts/eval-retrieval.ts: ciwa -> ciwa-ar content alias. The matcher
  is whitespace-delimited, so the bare fixture term can never match the
  scale's hyphenated written name; canary runs #50/#51 ranked the
  CIWA-Ar dosing-table region top-5 yet the gate reported a miss.
- tests/helpers/zero-touch.ts: shared stubZeroTouchPoints helper
  replacing the six inline copies from #995 (review follow-up).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EXsJcLrbZUXwnBeG91cVo9
@supabase

supabase Bot commented Jul 20, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 98d18575-a7a1-493a-8ecd-9249f12d1159

📥 Commits

Reviewing files that changed from the base of the PR and between 9082462 and 549bb97.

📒 Files selected for processing (12)
  • docs/branch-review-ledger.md
  • playwright.config.ts
  • scripts/eval-retrieval.ts
  • src/components/clinical-dashboard/master-search-header.tsx
  • tests/helpers/zero-touch.ts
  • tests/ui-accessibility.spec.ts
  • tests/ui-formulation.spec.ts
  • tests/ui-pwa.spec.ts
  • tests/ui-smoke.spec.ts
  • tests/ui-stress.spec.ts
  • tests/ui-tools.spec.ts
  • tests/ui-universal-search.spec.ts

📝 Walkthrough

Walkthrough

The PR updates Playwright service-worker and touch-capability setup, centralizes WebKit test initialization, strengthens UI focus and state assertions, adds CIWA retrieval aliases, changes desktop mode-menu Tab handling, and records the run-4012 review remainder.

Changes

Run-4012 remainder

Layer / File(s) Summary
Service worker test scoping
playwright.config.ts, tests/ui-pwa.spec.ts
Service workers are blocked by default and explicitly enabled for the PWA spec.
Shared zero-touch capability setup
tests/helpers/zero-touch.ts, tests/ui-*.spec.ts
A shared helper stubs Navigator.maxTouchPoints and replaces duplicated setup in UI specs.
UI interaction validation
tests/ui-accessibility.spec.ts, tests/ui-formulation.spec.ts, tests/ui-smoke.spec.ts
Tests add a WebKit forced-colors skip, verify formulation step-nav state, and skip scrollable focus targets during mobile keyboard navigation.
Application behavior and review tracking
src/components/clinical-dashboard/master-search-header.tsx, scripts/eval-retrieval.ts, docs/branch-review-ledger.md
Desktop mode menus close on Tab, CIWA aliases include ciwa-ar, and the review ledger gains the run-4012 remainder entry.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested labels: codex

Suggested reviewers: claude

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/clinical-kb-pwa-review-asi3wb

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install timed out. The project may have too many dependencies for the sandbox.


Comment @coderabbitai help to get the list of available commands.

@BigSimmo
BigSimmo marked this pull request as ready for review July 20, 2026 15:28
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@BigSimmo
BigSimmo merged commit 313baa3 into main Jul 20, 2026
17 checks passed
@BigSimmo
BigSimmo deleted the claude/clinical-kb-pwa-review-asi3wb branch July 20, 2026 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants